Fix memory tracking of ES|QL sample agg#128467
Merged
jan-elastic merged 3 commits intoelastic:mainfrom May 27, 2025
Merged
Conversation
elasticsearchmachine
added
needs:triage
jan-elastic
added
>bug
>test-failure
elasticsearchmachine
added
needs:risk
Collaborator
|
Pinging @elastic/ml-core (Team:ML) |
ivancea
approved these changes
May 26, 2025
Comment on lines
99
to
101
| BytesRefBlock bytesRefBlock = (BytesRefBlock) block; | ||
| try ($Type$Block.Builder $type$Block = driverContext.blockFactory().new$Type$BlockBuilder(bytesRefBlock.getPositionCount())) { | ||
| try ( | ||
| block; |
Contributor
There was a problem hiding this comment.
Nit: What about moving the cast inside the try? So we avoid this extra block;. Not much of an improvement, but I think it makes the intent a bit clearer ("Now we'll consume this block, which is a BytesRefBlock"), and we use both variables from here on
Contributor
Author
There was a problem hiding this comment.
SGTM
Maybe it's slightly obscure that handles closing the original block.
Anyway, it also conveniently solves my spotless/autogen issue (see the autocommit). Depending the the strlen of the type (Int or BytesRef), this block should go on 1 or 2 lines right now.
jan-elastic
added a commit
to jan-elastic/elasticsearch
that referenced
this pull request
Jun 18, 2025
* Fix memory tracking of ES|QL sample agg * [CI] Auto commit changes from spotless * polish code --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
jan-elastic
added a commit
to jan-elastic/elasticsearch
that referenced
this pull request
Jun 18, 2025
* Fix memory tracking of ES|QL sample agg * [CI] Auto commit changes from spotless * polish code --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
jan-elastic
added a commit
to jan-elastic/elasticsearch
that referenced
this pull request
Jun 18, 2025
* Fix memory tracking of ES|QL sample agg * [CI] Auto commit changes from spotless * polish code --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
jan-elastic
added a commit
to jan-elastic/elasticsearch
that referenced
this pull request
Jun 19, 2025
* Fix memory tracking of ES|QL sample agg * [CI] Auto commit changes from spotless * polish code --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
jan-elastic
added a commit
that referenced
this pull request
Jun 19, 2025
* ES|QL SAMPLE aggregation function (#127629) * ES|QL SAMPLE aggregation function * [CI] Auto commit changes from spotless * ThreadLocalRandom -> SplittableRandom * Update docs/changelog/127629.yaml * fix yaml test * Add SampleTests * docs + example * polish code * mark generated imports * comment with algorith description * use Randomness.get() * close properly * type checks * reuse hash * regen some files * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> * Fix + unmute SampleTests (#127959) * Fix memory tracking of ES|QL sample agg (#128467) * Fix memory tracking of ES|QL sample agg * [CI] Auto commit changes from spotless * polish code --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> * ESQL: Unclean generated imports (#127723) This removes a ton of the tricky juggling we do for generated java files to keep the imports in order. Instead, we just live with them being out of order a little. It's not great, but it's so so so much easier than the terrible juggling we had been doing. * ESQL: Disable format checks on generated imports (#127648) This builds the infrastructure to disable spotless and some checkstyle rules on generated imports. This works around the most frustrating part of ESQL's string template generated files - the imports. It allows unused and out of order imports. This can let us remove a lot of cumbersome, tricky, and fairly useless `$if$` blocks from the templates. --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Nik Everett <nik9000@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #128024